feat(nexus): Early Nexus handler support #1708

bergundy · 2025-05-08T01:26:29Z

This PR is blocked on publishing an initial version of the nexus-rpc package.
There is a TODO to figure out HandlerError and OperationError message rehydration, pending discussion.
Interceptors not yet implemented.
WorkflowRunOperation and getClient() not implemented for the @temporalio/nexus package.
Tests use the HTTP API directly in lieu of a workflow caller or strongly typed client, we can refactor those later.

mjameswh · 2025-05-09T18:26:21Z

packages/common/src/internal-non-workflow/codec-helpers.ts

@@ -91,6 +91,17 @@ export async function decodeArrayFromPayloads(
  return arrayFromPayloads(payloadConverter, await decodeOptional(payloadCodecs, payloads));
 }

+/**
+ * Decode `payloads` and then return {@link arrayFromPayloads}`.


Docstring is wrong

mjameswh · 2025-05-09T19:48:43Z

packages/worker/src/worker-options.ts

+    if (nexusServices.has(s.name)) {
+      throw new TypeError(`Duplicate registration of nexus service ${s.name}`);
+    }
+    const ops = new Map<string, nexus.OperationHandler<any, any> | nexus.SyncOperationHandler<any, any>>();


Should that be part of the Nexus SDK? Logic is trivial at present, but there I expect there will be some normalization work to be done at a later point (e.g. once we add decorators) which we wouldn't to diverge from one Nexus implementation to another. This also seems to be what we did in Java, and kind-of what we did in Go.

Yeah, we discussed moving this to the Nexus SDK with Chad yesterday. I will do that.

Oh, I missed that part of the conversation.

I will do that.

I only wanted to confirm that we agree on direction. I can do it myself if you don't have time/prefer not to do it (same apply to all suggestions I make on this PR).

mjameswh · 2025-05-09T20:21:29Z

packages/test/src/test-nexus-handler.ts

+import getPort from 'get-port';
+import * as nexus from 'nexus-rpc';
+import * as protoJsonSerializer from 'proto3-json-serializer';
+import * as temporalnexus from '@temporalio/nexus';


Dependency is missing in package.json and tsconfig.json

I will fix.

mjameswh · 2025-05-09T21:24:01Z

packages/test/src/test-nexus-handler.ts

+  });
+
+  Runtime.install({ logger });
+  t.context.httpPort = await getPort();


TODO(JWH): This will be flaky in CI due to port collisions, and users will often need to do that themselves. Move this to Core.

mjameswh · 2025-05-09T21:52:05Z

packages/test/src/test-nexus-handler.ts

+test.beforeEach(async (t) => {
+  const taskQueue = t.title + randomUUID();
+  const { env } = t.context;
+  const response = await env.connection.operatorService.createNexusEndpoint({


I understand this is the way for now, but what's the intent regarding this? Do we expect users to have to do the same in their own Temporal+Nexus tests?

That's a good question. in Java we have a shortcut, we should probably make it easier in the test environment for Core based SDKs. @dandavison, wondering if you considered this in Python.

No, I have not considered APIs for user Temporal Nexus testing yet; I'll add a TODO to the code. In my tests I have a helper function like this for creating Nexus endpoints.

I think we need an issue to track this.

👍 Created one for Python SDK-3837 "Python Nexus user testing utilities and docs"

I would have made this a features issue with checkboxes for each SDK but 🤷, thanks!

mjameswh · 2025-05-09T22:33:50Z

packages/worker/src/nexus.ts

+      const input = await decodeFromPayload(this.dataConverter, payload);
+      if (typeof this.handler === 'function') {
+        const handler = this.handler as nexus.SyncOperationHandler<unknown, unknown>;
+        const output = await this.invokeUserCode('startOperation', handler.bind(undefined, input, options));


Would be preferable to add some defensive checks here to prevent a user from returning a HandlerStartOperationResultSync or HandlerStartOperationResultAsync from their SyncOperationHandler. Sounds like an easy error to make. But I don't think there's currently a safe way of doing this.

dandavison · 2025-05-13T22:05:04Z

packages/worker/src/nexus.ts

+    options: nexus.StartOperationOptions
+  ): Promise<coresdk.nexus.INexusTaskCompletion> {
+    try {
+      const input = await decodeFromPayload(this.dataConverter, payload);


If the data converter fails here, I assume it should result in a 400 BAD_REQUEST Nexus HTTP response, right? If that's correct can you confirm that it does?

It's a test worth adding. Thanks!

- This PR is blocked on publishing an initial version of the `nexus-rpc` package. - There is a TODO to figure out HandlerError and OperationError message rehydration, pending discussion. - Interceptors not yet implemented. - `WorkflowRunOperation` and `getClient()` not implemented for the `@temporalio/nexus` package. - Tests use the HTTP API directly in lieu of a workflow caller or strongly typed client, we can refactor those later.

dandavison · 2025-05-17T22:22:18Z

packages/test/src/test-nexus-link-converter.ts

+import {
+  convertWorkflowEventLinkToNexusLink,
+  convertNexusLinkToWorkflowEventLink,
+} from '@temporalio/worker/lib/nexus/link-converter';


Suggested change

} from '@temporalio/worker/lib/nexus/link-converter';

} from '@temporalio/nexus/lib/link-converter';

dandavison · 2025-05-18T18:06:42Z

packages/nexus/src/context.ts

+) => Promise<WorkflowHandle<O>>;
+
+export class WorkflowRunOperation<I, O> implements nexus.OperationHandler<I, O> {
+  constructor(readonly handler: WorkflowRunOperationHandler<I, O>) {}


Given that WorkflowRunOperation implements OperationHandler, isn't there an argument that WorkflowRunOperation should be called WorkflowRunOperationHandler and that the function passed into the constructor (type currently called WorkflowRunOperationHandler) should be called startHandler: WorkflowRunOperationStartHandler?

dandavison · 2025-05-26T17:31:37Z

packages/nexus/src/token.ts

+  }
+  if (!token.wid) {
+    throw new TypeError('invalid workflow run token: missing workflow ID (wid)');
+  }


I see that Go and TS don't validate the 'ns' field. Is that deliberate?

dandavison · 2025-05-26T17:36:30Z

packages/nexus/src/token.ts

+// Currently, we only have one type of operation token: WorkflowRun.
+type OperationTokenType = typeof OPERATION_TOKEN_TYPE_WORKFLOW_RUN;
+
+interface WorkflowRunOperationToken {


Will you still be happy with this name when we have other operations such as WorkflowSignalWithStartOperation and WorkflowUpdateWithStartOperation that start workflows? Or is it better to give it a slightly more general name that indicates that it represents a workflow but is agnostic regarding how that workflow was started?

Similarly, OPERATION_TOKEN_TYPE_WORKFLOW might be a better name ("RUN" does not mean a workflow run here; it's just a wid hence extends over CAN)

…verdaccio nor official npm)

dandavison · 2025-06-20T14:25:43Z

packages/test/src/test-nexus-handler.ts

+              workflowId,
+              // To test attaching multiple callers to the same operation.
+              workflowIdConflictPolicy: 'USE_EXISTING',
+            });


The design proposed here is that, when a user wants to start a workflow that is the backing execution for their Nexus operation, they must call a special temporalnexus.startWorkflow instead of the usual client.startWorkflow. The "special" version takes care of

propagating nexus request ID to StartWorkflow request ID

establishing bidirectional linkage

I'm going to note some downsides with this design; no implication intended that other designs are free of downsides.

The user may not know to do this, and obtain a normal Temporal client and start their workflow in the usual way via client.startWorkflow.

The user may write a Nexus operation that starts multiple workflows, intending only the final one to be the "backing" workflow. But if they use temporalnexus.startWorkflow for more than one workflow, they will get unexpected behavior.

I'm partly mentioning this because I had been using a similar design for Python (in Python I was hoping not to introduce a second start_workflow). But there are similar challenges.

So far we've been saying that it's better for the user to be explicit about the workflow that they intend to use to back the operation. Sure, there are cases where users may try to call this function twice in a handler and we can decide whether it's undefined behavior or a runtime error.

I don't think implicitly doing all of the things the "special" function does in client.startWorkflow is the right approach. And note that the context here is passed explicitly as we've discussed in the long explicit vs. implicit context discussion, let's not go back on that.

I like this version the best as it is consistent with Go, and allows a user to intercept failures.

FTR, this helper also propagates the callbacks and populates onConflictOptions when USE_EXISTING is used to attach a callback to an already running workflow.

I agree we don't want to do the "special" things implicitly in client.startWorkflow.

I think what we want to do is almost exactly what you're doing, but instead of returning WorkflowHandle<OutputT>, return a type like WorkflowOperationToken<OutputT>.

For Python's solution to this, see https://github.com/temporalio/sdk-python/pull/813/files#r2160553793.

With what you've got currently, the user could easily make a mistake and use the normal client.startWorkflow, thinking that they are creating a nexusified workflow. And that would be very bad: there would be no type-check-time error and no request-time error; they will not be alerted to their mistake until they see that their operation result is never delivered and there is no bidirectional linking.

But having temporalnexus.startWorkflow return WorkflowOperationToken<OutputT> solves that: they still have the familiar startWorkflow interface, but it's impossible for the user to make the mistake in Typescript due to the type error.

These are the requirements that I believe we should be designing for here:

A Nexus operation handler must be able to start a nexusified workflow:
a. delivers the final Nexus operation result
b. has bidirectional linking
c. Nexus request ID propagated into the StartWorkflow request

A Nexus operation handler must also be able to start “normal” workflows.

It should not be possible for a user who is trying to do (1) to get confused and accidentally do (2) instead.

The above must all hold whether a user is using the “shorthand” form (WorkflowRunOperation) or manually implementing an operation handler.

bergundy requested a review from a team as a code owner May 8, 2025 01:26

mjameswh reviewed May 9, 2025

View reviewed changes

dandavison reviewed May 13, 2025

View reviewed changes

bergundy added 8 commits May 14, 2025 20:02

Use the Nexus SDK ServiceRegistry

0490e7b

Upgrade core, TODO fix protos and versioning behavior

be83b0d

Link converter

2150a48

Add worker/src/nexus directory

3dcc150

Lint

5e4a5c3

getClient() and fix versioning strategy

d29dee1

WorkflowRunOperation

29e6c12

bergundy force-pushed the nexus branch from c8793dc to 29e6c12 Compare May 16, 2025 23:34

Merge remote-tracking branch 'origin/main' into nexus

244cbbb

dandavison reviewed May 17, 2025

View reviewed changes

dandavison reviewed May 18, 2025

View reviewed changes

bergundy added 6 commits May 19, 2025 09:43

Fix build

0e6dad1

Cleanup

64cee58

Nexus workflow client

ab2645a

Fail requests with BAD_REQUEST if payload decoding fails

a627bfd

Format code

69d01c0

Merged context and options

ebf1311

dandavison reviewed May 26, 2025

View reviewed changes

bergundy and others added 2 commits May 28, 2025 13:21

OperationContext

9ed2ddf

Merge branch 'main' into pr/bergundy/1708

79c9303

mjameswh changed the title ~~Nexus handler near complete implementation~~ feat(nexus): Add early Nexus handler support Jun 4, 2025

mjameswh changed the title ~~feat(nexus): Add early Nexus handler support~~ feat(nexus): Early Nexus handler support Jun 4, 2025

mjameswh added 10 commits June 5, 2025 14:12

stash - fixing build and test issues

7a836da

Try temp solution to get CI running

64aee0a

Try temp solution to get CI running - Take 2

386fac6

Get CI running - Fix some issues

86d6575

Fixes failing tests

a442b2c

Fixes failing test on windows

35ef1b9

Fixes failing tests on Node 18 and Windows

d90ef24

temporalio/nexus package was not being exported to npm repo (neither …

40d6b52

…verdaccio nor official npm)

Fix race in test causing flakyness

0728ae2

Trivial renames of Nexus APIs

9073514

dandavison reviewed Jun 20, 2025

View reviewed changes

	} from '@temporalio/worker/lib/nexus/link-converter';
	} from '@temporalio/nexus/lib/link-converter';

feat(nexus): Early Nexus handler support #1708

Are you sure you want to change the base?

feat(nexus): Early Nexus handler support #1708

Uh oh!

Conversation

bergundy commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dandavison May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dandavison Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dandavison Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dandavison Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dandavison May 13, 2025 •

edited

Loading

dandavison Jun 20, 2025 •

edited

Loading

dandavison Jun 21, 2025 •

edited

Loading

dandavison Jun 22, 2025 •

edited

Loading